Skip to content

Fix gRPC BoardListWatch crashing when installing/uninstalling a core/lib #1460

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 22, 2021

Conversation

silvanocerza
Copy link
Contributor

Please check if the PR fulfills these requirements

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • The PR follows
    our contributing guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • UPGRADING.md has been updated with a migration guide (for breaking changes)
  • What kind of change does this PR introduce?

Fixes a bug in the gRPC interface.

  • What is the current behavior?

Calling one of the following gRPC function:

  • PlatformInstall
  • PlatformUninstall
  • PlatformUpgrade
  • LibraryInstall
  • LibraryUninstall
  • LibraryUpgradeAll

crashes the arduino-cli daemon if the BoardListWatch process for the instance used to call the aforementioned functions is running.

  • What is the new behavior?

Calling one of the following gRPC function:

  • PlatformInstall
  • PlatformUninstall
  • PlatformUpgrade
  • LibraryInstall
  • LibraryUninstall
  • LibraryUpgradeAll

stops the BoardListWatch process for the instance used to call the aforementioned functions, the connected clients will be notified the BoardListWatch process is quitting.

If the client will need to restart the BoardListWatch process it must wait for the above functions to return a response.

Nope.

  • Other information:

This bug is caused by the internal workings of the Arduino CLI daemon.
gRPC clients using the CLI must first Create a new instance that is used to keep track of installed platforms, tools, discoveries and libraries, that instance is populated with data using the Init function. Since there can be multiple clients connection via gRPC we might have multiple instances internally to keep track of.

The Init function can also be used on an already initialized instance so the first thing we do is "clean" that instance, that means also stopping all the pluggable discovery processes that are running, doing that while the BoardListWatch process is running causes the bug being fixed by this PR.

The best possible solution would have been to notify the BoardListWatch process that the discoveries are quitting and act accordingly by restarting it after the initialization step has finished but that would require a big paradigm change in the internal logic of the CLI that we can't bear as of now, thus this solution.


See how to contribute

@silvanocerza silvanocerza requested a review from a team September 21, 2021 13:46
@silvanocerza silvanocerza self-assigned this Sep 21, 2021
@silvanocerza silvanocerza force-pushed the scerza/fix-grpc-board-list-watch branch from 753bef8 to dae732e Compare September 21, 2021 13:59
@silvanocerza silvanocerza force-pushed the scerza/fix-grpc-board-list-watch branch from dae732e to 5d3bbb7 Compare September 22, 2021 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: gRPC Related to the gRPC interface type: imperfection Perceived defect in any part of project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants